Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix rllib #140

Merged
merged 32 commits into from
Aug 3, 2023
Merged

Fix rllib #140

merged 32 commits into from
Aug 3, 2023

Conversation

edbeeching
Copy link
Owner

@edbeeching edbeeching commented Jul 27, 2023

Should resolve #139

  • Updates RLLIB wrappers to support latest version.
  • Updates setup to install correct version of gymnasium for rllib
  • Make cleanrl work again (use gymnasium instead of gym)
  • Make hp tuning work again (use gymnasium instead of gym)

Needs install from new venv with pip install godot-rl[rllib]

@visuallization
Copy link
Collaborator

@edbeeching haha oh noes! I was working on the same thing as you :D

#141

@edbeeching
Copy link
Owner Author

edbeeching commented Jul 27, 2023

@edbeeching haha oh noes! I was working on the same thing as you :D

#141

Doh! Luckily it is not a big change. We should be a bit more organized with assigning things in the future. It seems we made some complementary changes, if you want to pick up the ones from my branch and add them to yours? I will then close mine and we can get yours merged.

@visuallization
Copy link
Collaborator

I think yours is nicer (no check if obs exists and making infos a list), so we should go with yours. The only thing I noticed is that we apparently have to keep the fixed numpy==1.23.5 version. At least for me it throws an error if we don't keep it.

image

How is it for you? Maybe it is just realted of my godot project which uses an older version of the plugin.

@visuallization
Copy link
Collaborator

@edbeeching haha oh noes! I was working on the same thing as you :D
#141

Doh! Luckily it is not a big change. We should be a bit more organized with assigning things in the future. It seems we made some complementary changes, if you want to pick up the ones from my branch and add them to yours? I will then close mine and we can get yours merged.

Agreed! next time, the one who wants to tackle the problem, should just claim so in the issue.

@edbeeching
Copy link
Owner Author

I think yours is nicer (no check if obs exists and making infos a list), so we should go with yours. The only thing I noticed is that we apparently have to keep the fixed numpy==1.23.5 version. At least for me it throws an error if we don't keep it.

image

How is it for you? Maybe it is just realted of my godot project which uses an older version of the plugin.

It is strange I don't see to have problem with numpy, but I am running on Linux so perhaps that is it?

I will add your docs changes to my PR.

@edbeeching
Copy link
Owner Author

@visuallization would you mind installing from scratch with this PR + rllib? I want to be sure that we need to set the numpy version.

@visuallization
Copy link
Collaborator

I think yours is nicer (no check if obs exists and making infos a list), so we should go with yours. The only thing I noticed is that we apparently have to keep the fixed numpy==1.23.5 version. At least for me it throws an error if we don't keep it.
image
How is it for you? Maybe it is just realted of my godot project which uses an older version of the plugin.

It is strange I don't see to have problem with numpy, but I am running on Linux so perhaps that is it?

I will add your docs changes to my PR.

I am also on Linux now - so that's not it. It might be related to godotrl plugin - i will try upgrding the plugin and check again

@visuallization
Copy link
Collaborator

@visuallization would you mind installing from scratch with this PR + rllib? I want to be sure that we need to set the numpy version.

I did just that and it is not working for me :(. Not even after using the godot_rl addon from main. which addon version are you using? And what is the numpy version you are using? pip show numpy

@edbeeching
Copy link
Owner Author

edbeeching commented Jul 27, 2023

I did just that and it is not working for me :(. Not even after using the godot_rl addon from main. which addon version are you using? And what is the numpy version you are using? pip show numpy

Name: numpy
Version: 1.25.1
Summary: Fundamental package for array computing in Python
Home-page: https://www.numpy.org
Author: Travis E. Oliphant et al.
Author-email: 
License: BSD-3-Clause
Location: /home/edward/play/godot/godot_rl/godot_rl_agents/venv/lib/python3.10/site-packages
Requires: 
Required-by: contourpy, godot-rl, gym, Gymnasium, huggingface-sb3, imageio, matplotlib, onnx, onnxruntime, pandas, pyarrow, PyWavelets, ray, scikit-image, scipy, stable-baselines3, tensorboard, tensorboardX, tifffile

python version 3.10.9

@visuallization
Copy link
Collaborator

hmm I have python 3.9.16

@visuallization
Copy link
Collaborator

Hmm maybe it is related on how my actions look.
2 continuos and 1 discrete. my env is very similar to jumperhard. Does jumperhard work for you with latest numpy version?

@visuallization
Copy link
Collaborator

@edbeeching Okay so JumperHard does also not working for me with the current numpy version.
Which environment are you using for testing? Is it maybe related to envs which are mixing continuous and discrete spaces?

@visuallization
Copy link
Collaborator

visuallization commented Jul 27, 2023

@edbeeching Okay so yes the actions are exaclty the problem, because the array consists of different types and sizes. see;

Reproducing in plain python:
image

This is how the action look for jumperhard, whcih fails with latest numpy version:
image

@visuallization
Copy link
Collaborator

@edbeeching fixed: 20c4df7

@Ivan-267
Copy link
Collaborator

Ivan-267 commented Jul 27, 2023

@edbeeching fixed: 20c4df7

Nice catch, this is what I get when trying something similar:

np.array([1,[2,3]])
<stdin>:1: VisibleDeprecationWarning: Creating an ndarray from ragged nested sequences (which is a list-or-tuple of lists-or-tuples-or ndarrays with different lengths or shapes) is deprecated. If you meant to do this, you must specify 'dtype=object' 

when creating the ndarray with numpy '1.21.2' on a local python 3.9 installation. I guess they later changed the warning to an error.

@visuallization
Copy link
Collaborator

@edbeeching Your implementation does not seem to take num_workers correctly into account. Check this out: Both versions where run with the same config but your implementation took around 20m while mine took around 10m. Not sure what's going on. Rerunning the experiment to make sure I did not mix anything up.

image

@visuallization
Copy link
Collaborator

@edbeeching Your implementation does not seem to take num_workers correctly into account. Check this out: Both versions where run with the same config but your implementation took around 20m while mine took around 10m. Not sure what's going on. Rerunning the experiment to make sure I did not mix anything up.

image

Ah okay it is because with your implementation we have to set --speedup explicitly via terminal

@visuallization visuallization mentioned this pull request Jul 27, 2023
@visuallization
Copy link
Collaborator

visuallization commented Jul 28, 2023

@edbeeching I fixed the install step in the tests but now the actual test seems to timeout for rllib.

Check this regarding fixing the wheel version 🤷 freqtrade/freqtrade#8376

@Ivan-267
Copy link
Collaborator

Ivan-267 commented Jul 28, 2023

About fixing versions, I also think that it would help with stability, but then we may have to make sure that the version (or version ranges) specified are compatible for most operating systems / Python versions that we want to support. Maybe that's the case for all or most packages, just something to consider.

The last test yesterday also did start successfully but was taking too long, although now it should install whichever rllib version is set in the requirements.

Edit: I noticed that now in the test, sb3 1.8 is installed when using pip install .[rllib]

https://github.com/edbeeching/godot_rl_agents/actions/runs/5690825628/job/15424893195

Successfully installed PyWavelets-1.4.1 aiosignal-1.3.1 attrs-23.1.0 click-8.1.6 dm-tree-0.1.8 frozenlist-1.4.0 godot-rl-0.6.1 gym-0.21.0 gymnasium-0.26.3 gymnasium-notices-0.0.1 imageio-2.31.1 importlib-metadata-4.13.0 jsonschema-4.18.4 jsonschema-specifications-2023.7.1 lazy_loader-0.3 lz4-4.3.2 markdown-it-py-3.0.0 mdurl-0.1.2 msgpack-1.0.5 pyarrow-12.0.1 pygments-2.15.1 ray-2.6.1 referencing-0.30.0 rich-13.4.2 rpds-py-0.9.2 scikit-image-0.21.0 scipy-1.11.1 stable-baselines3-1.8.0 tensorboardX-2.6.1 tifffile-2023.7.18 typer-0.9.0

Locally I get this error with Conda and Python 3.9 or 3.10:

Collecting stable-baselines3 (from godot-rl==0.6.1)
  Using cached stable_baselines3-1.8.0-py3-none-any.whl (174 kB)
Collecting gym==0.21 (from stable-baselines3->godot-rl==0.6.1)
  Using cached gym-0.21.0.tar.gz (1.5 MB)
  Preparing metadata (setup.py) ... error
  error: subprocess-exited-with-error

  × python setup.py egg_info did not run successfully.
  │ exit code: 1
  ╰─> [1 lines of output]
      error in gym setup command: 'extras_require' must be a dictionary whose values are strings or lists of strings containing valid project/version requirement specifiers.
      [end of output]

And if I try

python -m pip install --upgrade pip wheel==0.38.4

It just says:

Requirement already satisfied: pip in miniconda3\envs\rllib\lib\site-packages (23.2.1)
Requirement already satisfied: wheel==0.38.4 in miniconda3\envs\rllib\lib\site-packages (0.38.4)

It will install with the solution here:
openai/gym#3176 (comment)

I had to use:

python.exe -m pip install setuptools==65.5.0 pip==21

Before pip install .[rllib]

That does make the install process a bit complicated for rllib, but it may be possible to just use pip install ray[rllib] instead for now at least on local PC to simplify that step (there's no need to reinstall the older version of sb3).

When I run the examples install script and then try: pytest .\tests\test_rllib.py I get this error:

(RolloutWorker pid=3160) OSError: [WinError 216] This version of %1 is not compatible with the version of Windows you're running. Check your computer's system information and then contact the software publisher [repeated 9x across cluster]

I wasn't able to start the binaries either locally, even with trying gdrl.env_from_hub -r edbeeching/godot_rl_JumperHard the game exe seems incompatible with 64 bit windows.

Here is the entire content of JumperHard.exe after the download:

version https://git-lfs.github.com/spec/v1
oid sha256:784e2e3b2022a9421df07c764fc394ff0576147b16d186b9ed17fb725e96d4d4
size 68805120

After rebuilding the example in Godot, the test_rllib.py test does pass locally (quickly) but there are multiple warnings:

================================================================================================ warnings summary ================================================================================================
tests/test_rllib.py::test_rllib_training
  C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\ray\tune\logger\tensorboardx.py:41: DeprecationWarning: `np.bool8` is a deprecated alias for `np.bool_`.  (Deprecated NumPy 1.24)
    VALID_NP_HPARAMS = (np.bool8, np.float32, np.float64, np.int32, np.int64)

tests/test_rllib.py::test_rllib_training
  C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\ray\tune\logger\tensorboardx.py:169: DeprecationWarning: `np.bool8` is a deprecated alias for `np.bool_`.  (Deprecated NumPy 1.24)
    VALID_NP_HPARAMS = (np.bool8, np.float32, np.float64, np.int32, np.int64)

tests/test_rllib.py::test_rllib_training
  C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\gym\envs\registration.py:250: DeprecationWarning: SelectableGroups dict interface is deprecated. Use select.
    for plugin in metadata.entry_points().get(entry_point, []):

tests/test_rllib.py::test_rllib_training
  C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\torch\utils\tensorboard\__init__.py:4: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
    if not hasattr(tensorboard, "__version__") or LooseVersion(

tests/test_rllib.py::test_rllib_training
  C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\torch\utils\tensorboard\__init__.py:6: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
    ) < LooseVersion("1.15"):

tests/test_rllib.py::test_rllib_training
  C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\ray\tune\tune.py:258: UserWarning: Passing a `local_dir` is deprecated and will be removed in the future. Pass `storage_path` instead or set the `RAY_AIR_LOCAL_CACHE_DIR` environment variable instead.
    warnings.warn(

tests/test_rllib.py::test_rllib_training
  C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\ray\tune\tune.py:735: DeprecationWarning: checkpoint_freq is deprecated and will be removed. use checkpoint_config.checkpoint_frequency instead.
    warnings.warn(

tests/test_rllib.py::test_rllib_training
  C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\ray\tune\tune.py:742: DeprecationWarning: checkpoint_at_end is deprecated and will be removed. use checkpoint_config.checkpoint_at_end instead.
    warnings.warn(

tests/test_rllib.py::test_rllib_training
  C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\gymnasium\spaces\box.py:127: UserWarning: WARN: Box bound precision lowered by casting to float32
    logger.warn(f"Box bound precision lowered by casting to {self.dtype}")

tests/test_rllib.py::test_rllib_training
  C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\gymnasium\utils\passive_env_checker.py:141: UserWarning: WARN: The obs returned by the `reset()` method was expecting numpy array dtype to be float32, actual type: float64
    logger.warn(

tests/test_rllib.py::test_rllib_training
  C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\gymnasium\utils\passive_env_checker.py:165: UserWarning: WARN: The obs returned by the `reset()` method is not within the observation space.
    logger.warn(f"{pre} is not within the observation space.")

tests/test_rllib.py: 73 warnings
  C:\Users\computer\miniconda3\envs\rllib\lib\site-packages\tensorboardX\summary.py:153: DeprecationWarning: Conversion of an array with ndim > 0 to a scalar is deprecated, and will error in future. Ensure you extract a single element from your array before performing this operation. (Deprecated NumPy 1.25.)
    scalar = float(scalar)

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
======================================================================================== 1 passed, 84 warnings in 46.77s =========================================================================================

@visuallization
Copy link
Collaborator

The last test yesterday also did start successfully but was taking too long

@Ivan-267 Oh really? Maybe I misslooked but it seemed like it would still throw the same pip install error Ed reported. Anyways if your version works, please commit. :)

@Ivan-267
Copy link
Collaborator

Ivan-267 commented Jul 28, 2023

The last test yesterday also did start successfully but was taking too long

@Ivan-267 Oh really? Maybe I misslooked but it seemed like it would still throw the same pip install error Ed reported. Anyways if your version works, please commit. :)

I accidentally made it as two commits (from two single comment suggestions), but the second one did pass that part (not everything):

It just shows the same error message as I got locally (but it doesn't halt the test):
https://github.com/edbeeching/godot_rl_agents/actions/runs/5685626539/job/15422792046

godot-rl 0.6.1 requires stable-baselines3, which is not installed.
Successfully installed PyWavelets-1.4.1 aiosignal-1.3.1 attrs-23.1.0 click-8.1.6 dm-tree-0.1.8 frozenlist-1.4.0 gymnasium-0.26.3 gymnasium-notices-0.0.1 imageio-2.31.1 jsonschema-4.18.4 jsonschema-specifications-2023.7.1 lazy_loader-0.3 lz4-4.3.2 markdown-it-py-3.0.0 mdurl-0.1.2 msgpack-1.0.5 pyarrow-12.0.1 pygments-2.15.1 ray-2.6.1 referencing-0.30.0 rich-13.4.2 rpds-py-0.9.2 scikit-image-0.21.0 scipy-1.11.1 tensorboardX-2.6.1 tifffile-2023.7.18 typer-0.9.0

However I wouldn't say it works because the test is still taking too long as with your case. I'm not sure why, we could try changing some config and see if it makes a difference.

I was trying the test itself locally with pytest, but I found an issue with the example download step (for some reason it's not cloning the large files, e.g. the exe for me). However my local Windows environment may not be exactly the same as the test environment (or maybe I missed some step), as I see the test env downloads the examples just fine.

The passed test with warnings above was from pytest .\tests\test_rllib.py on my local system (after rebuilding the exe).

Just checking if it makes a difference
@Ivan-267
Copy link
Collaborator

Ivan-267 commented Jul 28, 2023

It looks like it is passing the rllib test now (similarly to as on my PC, with warnings).

I'm not sure if it was the timesteps, changing num workers to 1 (maybe the multiple workers cause some issue with test env? it works OK on my local setup), or some of the other changes that fixed it.

You could check the remaining error:

godot_rl/wrappers/sample_factory_wrapper.py:100: AttributeError
=========================== short test summary info ============================
FAILED tests/test_sample_factory.py::test_sample_factory_training - AttributeError: 'Namespace' object has no attribute 'seed'

@edbeeching
Copy link
Owner Author

Hey thanks to you both for continuing to work on this. I am travelling this weekend and will pick up on Monday.

@visuallization
Copy link
Collaborator

It looks like it is passing the rllib test now (similarly to as on my PC, with warnings).

I'm not sure if it was the timesteps, changing num workers to 1 (maybe the multiple workers cause some issue with test env? it works OK on my local setup), or some of the other changes that fixed it.

You could check the remaining error:

godot_rl/wrappers/sample_factory_wrapper.py:100: AttributeError
=========================== short test summary info ============================
FAILED tests/test_sample_factory.py::test_sample_factory_training - AttributeError: 'Namespace' object has no attribute 'seed'

@Ivan-267 that's awesome! Thanks for fixing the tests including the seeding issue - I forgot about the sample factory example env!

@edbeeching
Copy link
Owner Author

@visuallization or @Ivan-267 thanks for fixing the tests. I just pushed an update to the rllib installation instructions. If you can approve the PR then I will merge.

@visuallization
Copy link
Collaborator

visuallization commented Aug 3, 2023

@edbeeching @Ivan-267 Ray is updating gymnasium: ray-project/ray#37393 (comment). 🎉 Will be released in around 3 weeks. So we could keep an all option and fix the version for it, so it stays compatible in the future.

@edbeeching
Copy link
Owner Author

@visuallization , this is great. I will merge as is for now as there are a number of other fixes in this PR.

@edbeeching edbeeching merged commit 2725bc8 into main Aug 3, 2023
@edbeeching edbeeching deleted the fix-rllib branch August 3, 2023 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ray[rllib] does not work in godotrl 0.6.0
3 participants